Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make preferred-dir the default read/write directory for slash commands #881

Merged
merged 19 commits into from
Jul 9, 2024

Conversation

andrewfulton9
Copy link
Contributor

Closes #880

adds preferred_dir attribute and _output_dir property to BaseChatHandler and updates children handlers to use self._output_dir property in place of self.root_dir.

andrewfulton9 and others added 7 commits July 8, 2024 16:55
…ase enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@srdas srdas added the bug Something isn't working label Jul 8, 2024
@dlqqq
Copy link
Member

dlqqq commented Jul 8, 2024

@andrewfulton9 Thank you for opening this PR! I'm planning to give this a review by tomorrow.

@srdas
Copy link
Collaborator

srdas commented Jul 8, 2024

Tested the branch and it is working as expected:
Start Jupyter Lab : jupyter lab --preferred-dir='/Users/sanjivda/GitHub/TEST/'
Try both /export and /generate:
image
image
Also tested that when there is no preferred-dir, then it is written to the starting directory.

Also tested this with /learn -r arxiv <filename> to make sure the reference TeX file is stored in the preferred directory as well.
image
image

Copy link
Collaborator

@srdas srdas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and tested the new branch. Works as expected.
@andrewfulton9 Thanks for the nice contribution.

Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewfulton9 Thank you for implementing this and helping us clean up the backend! Love that we're unifying logic. Left you a minor comment below.

@srdas Thank you for testing & verifying this PR for me.

packages/jupyter-ai/jupyter_ai/chat_handlers/base.py Outdated Show resolved Hide resolved
Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewfulton9 Awesome work, thank you!

@dlqqq dlqqq merged commit 0d86ee8 into jupyterlab:main Jul 9, 2024
8 checks passed
@krassowski krassowski mentioned this pull request Jul 11, 2024
Marchlak pushed a commit to Marchlak/jupyter-ai that referenced this pull request Oct 28, 2024
jupyterlab#881)

* add preferred dir functionality to BaseChatHandler

* remove print

* cleanup

* Change /learn to use _output_dir instead of root dir

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add preferred dir functionality to BaseChatHandler

* remove print

* cleanup

* Change /learn to use _output_dir instead of root dir

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* change _output_dir to output_dir

* Update packages/jupyter-ai/jupyter_ai/chat_handlers/base.py

Co-authored-by: david qiu <[email protected]>

* add type hint

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: david qiu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

slash commands should read/write to preferred-dir instead of root-dir
3 participants